Skip to content

Fix slash command autofill broken after containerless rewrite#332

Merged
brendanlong merged 3 commits intomainfrom
claude/f7ce40a3-9dc7-46a2-9680-44fe38fe3088
Apr 10, 2026
Merged

Fix slash command autofill broken after containerless rewrite#332
brendanlong merged 3 commits intomainfrom
claude/f7ce40a3-9dc7-46a2-9680-44fe38fe3088

Conversation

@brendanlong
Copy link
Copy Markdown
Owner

Summary

  • When claude-runner.ts was rewritten for the containerless architecture, the slash command extraction and SSE emission logic was not migrated from the old agent-service
  • The UI infrastructure (PromptInput autocomplete, SSE subscriptions, tRPC endpoints) was fully intact, but the server never populated commands — so typing / did nothing
  • Fix: call q.supportedCommands() for rich metadata, extract slash_commands from system init message, merge both sources, and emit via sseEvents.emitCommands()
  • Added mergeSlashCommands() helper with 5 unit tests

Test plan

  • All 367 tests pass (pnpm test:run)
  • Start a session, type / in the prompt input — autocomplete dropdown should appear with commands like /compact, /commit, /review, etc.
  • Verify commands with descriptions (skills like /commit) show descriptions in the dropdown
  • Verify commands without descriptions (like /compact, /cost) still appear in the dropdown

🤖 Generated with Claude Code

When claude-runner.ts was rewritten to use the Agent SDK directly
in-process (removing the old agent-service container architecture),
the slash command extraction and SSE emission logic was not migrated.
The UI infrastructure (PromptInput autocomplete, SSE subscriptions)
was intact but the server never populated commands.

Fix by:
1. Calling q.supportedCommands() after starting a query to get rich
   command metadata (name, description, argumentHint)
2. Extracting slash_commands from the system init message and merging
   with the rich commands (init message has ALL commands, but as bare
   strings without descriptions)
3. Emitting merged commands via sseEvents.emitCommands()

Also adds mergeSlashCommands() helper with tests.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a mergeSlashCommands utility to combine rich command metadata from the SDK with command names extracted from system initialization messages. The review identified a potential race condition where the asynchronous SDK response could overwrite commands discovered during the initialization loop, and suggested an improvement to ensure the mergeSlashCommands function correctly handles duplicate entries in the input array.

Comment on lines +386 to +389
.then((commands) => {
supportedCommands = commands;
sseEvents.emitCommands(sessionId, commands);
log.info('Emitted supported commands from SDK', { sessionId, count: commands.length });
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There is a race condition between the asynchronous supportedCommands() call and the processing of the system init message in the loop. If the init message arrives first, supportedCommands is populated with the full list of names. When the SDK promise later resolves, it currently overwrites the variable with only the rich metadata subset, potentially losing commands that were only present in the init message (e.g., /compact, /cost). Merging the results in the .then block prevents this data loss.

      .then((commands) => {
        // Merge with any names already discovered from system init messages
        const currentNames = supportedCommands.map((c) => c.name);
        supportedCommands = mergeSlashCommands(commands, currentNames);
        sseEvents.emitCommands(sessionId, supportedCommands);
        log.info('Emitted supported commands from SDK', { sessionId, count: commands.length });
      })
References
  1. To prevent race conditions in methods with asynchronous initialization, set a processing/locked flag synchronously at the start of the method, before any await calls, to ensure concurrent invocations are rejected immediately.

Comment on lines +45 to +49
for (const name of slashCommandNames) {
if (!existingNames.has(name)) {
merged.push({ name, description: '', argumentHint: '' });
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation of mergeSlashCommands does not handle potential duplicates in the slashCommandNames array. If a name appears multiple times in the input array and is not already present in existingCommands, it will be added multiple times to the result. Updating the existingNames set within the loop ensures that each new command is only added once.

Suggested change
for (const name of slashCommandNames) {
if (!existingNames.has(name)) {
merged.push({ name, description: '', argumentHint: '' });
}
}
for (const name of slashCommandNames) {
if (!existingNames.has(name)) {
merged.push({ name, description: '', argumentHint: '' });
existingNames.add(name);
}
}

claude added 2 commits April 8, 2026 13:39
Address PR review feedback:
- Fix race condition: when supportedCommands() resolves after the
  system init message, merge with already-discovered names instead
  of overwriting them
- Fix duplicate handling: add names to the existingNames set during
  iteration to prevent duplicates from slashCommandNames array

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Cache discovered commands on SessionState so the getCommands
  endpoint returns them on page reload (not just via SSE)
- Use SystemInitContentSchema.safeParse() instead of manual
  property checks for type-safe init message parsing
- Replace length-based heuristic with actual set comparison
  to detect new commands accurately

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@brendanlong brendanlong merged commit 083b156 into main Apr 10, 2026
4 checks passed
@brendanlong brendanlong deleted the claude/f7ce40a3-9dc7-46a2-9680-44fe38fe3088 branch April 10, 2026 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants